Skip to content

Fix multiselect issue with section field - #2521

Open
AbdiTolesa wants to merge 21 commits into
masterfrom
issue-5229
Open

Fix multiselect issue with section field#2521
AbdiTolesa wants to merge 21 commits into
masterfrom
issue-5229

Conversation

@AbdiTolesa

@AbdiTolesa AbdiTolesa commented Oct 1, 2025

Copy link
Copy Markdown
Contributor

Fix https://github.com/Strategy11/formidable-pro/issues/5229

Test steps

  1. Add a few fields, a Section field and some fields into the section.
  2. Try multi selecting with ctrl + click on fields outside the Section and then try adding the Section field to the selected groups by clicking on the Section header.
  3. Confirm that the section field is selectable with other fields.
CleanShot.2025-10-01.at.16.26.33.mp4
CleanShot.2025-10-01.at.16.16.00.mp4

Summary by CodeRabbit

  • New Features
    • Added Ctrl/Meta-click support for selecting and deselecting field groups in the admin builder.
    • Updated the multiselect interface to display the current number of selected groups.
    • Prevented group-selection clicks from triggering standard field selection.

@coderabbitai

coderabbitai Bot commented Oct 1, 2025

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 26b8e095-4f74-4568-ad76-2fd9450a812d

📥 Commits

Reviewing files that changed from the base of the PR and between 889d6af and 34864c4.

📒 Files selected for processing (2)
  • js/formidable_admin.js
  • js/src/admin/admin.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • js/src/admin/admin.js

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Adds Ctrl/Meta-click handling to the admin field builder. Modifier-clicks toggle the containing field group, update the selected-group count and multiselect UI, and skip normal field selection. Plain clicks retain the existing behavior.

Changes

Admin builder field-group selection

Layer / File(s) Summary
Field-group selection toggle
js/src/admin/admin.js
Ctrl/Meta-clicks toggle the containing field group, update the selected-group count and multiselect state, and return before the normal field-selection action.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 34864

This localized change addresses section-field multiselect behavior, and no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

Suggested labels: run analysis, run tests

Suggested reviewers: crabcyborg

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: fixing multiselect behavior for Section fields.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-5229

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a24133c and 92a1567.

⛔ Files ignored due to path filters (2)
  • js/formidable-settings-components.js.map is excluded by !**/*.map, !**/*.map
  • js/formidable_dashboard.js.map is excluded by !**/*.map, !**/*.map
📒 Files selected for processing (1)
  • js/src/admin/admin.js (1 hunks)

Comment thread js/src/admin/admin.js Outdated
@lauramekaj1

Copy link
Copy Markdown
Contributor

Hi @AbdiTolesa,
Multi-selecting including the Section field works as expected.
However, when trying to deselect fields in a group selection (e.g., Ctrl + Click to remove one item from the group), it doesn't work properly since the field remains selected.
Please see screen recording for reference:
https://www.loom.com/share/1bd785331c9b44ffa884dd3c3bfa959e

@AbdiTolesa

Copy link
Copy Markdown
Contributor Author

Hi @AbdiTolesa,
Multi-selecting including the Section field works as expected.
However, when trying to deselect fields in a group selection (e.g., Ctrl + Click to remove one item from the group), it doesn't work properly since the field remains selected.

Hi @lauramekaj1 I just created a separate issue for that: https://github.com/Strategy11/formidable-pro/issues/6029
This PR is just intended to fix multi-select issues for fields inside Section fields.

@lauramekaj1

Copy link
Copy Markdown
Contributor

Hi @AbdiTolesa,

Multi-selecting including the Section field works as expected.

However, when trying to deselect fields in a group selection (e.g., Ctrl + Click to remove one item from the group), it doesn't work properly since the field remains selected.

Hi @lauramekaj1 I just created a separate issue for that: https://github.com/Strategy11/formidable-pro/issues/6029

This PR is just intended to fix multi-select issues for fields inside Section fields.

@AbdiTolesa ok so I validated that the issue in this PR is fixed. Thank you!

@AbdiTolesa
AbdiTolesa requested a review from truongwp October 9, 2025 17:02
@truongwp

Copy link
Copy Markdown
Contributor

@AbdiTolesa

Copy link
Copy Markdown
Contributor Author

@AbdiTolesa Does this cover this comment? Strategy11/formidable-pro#6029 (comment)

@truongwp The link takes to the issue created probably for the comment you were referring to, we are tracking that separately anyway.

@truongwp

Copy link
Copy Markdown
Contributor

@AbdiTolesa If I click on the first field, then hold Shift and click on a field inside a section, it selects all fields in the form. Please see this screencast:

Screen.Recording.2025-10-15.at.18.21.37.mov

@AbdiTolesa

Copy link
Copy Markdown
Contributor Author

@AbdiTolesa If I click on the first field, then hold Shift and click on a field inside a section, it selects all fields in the form. Please see this screencast:

@truongwp We are tracking that issue separately: https://github.com/Strategy11/formidable-pro/issues/6017

Comment thread js/src/admin/admin.js Outdated
@AbdiTolesa
AbdiTolesa requested a review from truongwp October 17, 2025 13:40

@truongwp truongwp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good now. Thanks @AbdiTolesa!

@truongwp

Copy link
Copy Markdown
Contributor

@AbdiTolesa Please resolve the conflicts with the master branch so we can merge it.

@truongwp
truongwp requested a review from Crabcyborg October 17, 2025 16:22

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
js/src/admin/admin.js (1)

5690-5694: Keep multi‑select UI in sync on Ctrl/Cmd click

You add the class and return, but don’t call the existing multi‑select sync flow. The popup/controls won’t appear until a later action. Reuse the same path as group clicks by invoking syncAfterMultiSelect immediately.

Apply this minimal change:

-		if ( e?.metaKey || e?.ctrlKey ) {
-			// Add the target to list of selected fields.
-			this.closest( 'ul' ).classList.add( 'frm-selected-field-group' );
-			return;
-		}
+		if ( e?.metaKey || e?.ctrlKey ) {
+			// Add the target to list of selected fields and sync UI state.
+			const ul = this.closest( 'ul.frm_sorting' ) || this.closest( 'ul' );
+			if ( ul ) {
+				ul.classList.add( 'frm-selected-field-group' );
+				const count = jQuery( '.frm-selected-field-group' ).length || 1;
+				syncAfterMultiSelect( count );
+				maybeHideFieldGroupMessage();
+			}
+			return;
+		}

Note: Ctrl/Cmd‑click deselect toggling is tracked in formidable-pro#6029 and can remain out of scope here. Please just verify the popup appears right after the first Ctrl/Cmd click.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a636cfa and 597dea4.

⛔ Files ignored due to path filters (1)
  • js/formidable-settings-components.js.map is excluded by !**/*.map, !**/*.map
📒 Files selected for processing (1)
  • js/src/admin/admin.js (1 hunks)

@AbdiTolesa
AbdiTolesa requested a review from truongwp October 23, 2025 10:55

@truongwp truongwp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good now. Thanks @AbdiTolesa!

@truongwp
truongwp requested review from Crabcyborg and removed request for Crabcyborg November 3, 2025 18:10
@Crabcyborg Crabcyborg added this to the 6.26 milestone Nov 3, 2025
@deepsource-io

deepsource-io Bot commented Feb 6, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in e4dc1b0...226d435 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
PHP Aug 19, 2026 3:39p.m. Review ↗
JavaScript Aug 19, 2026 3:39p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@AbdiTolesa
AbdiTolesa requested a review from Crabcyborg February 6, 2026 15:32
@Crabcyborg Crabcyborg modified the milestones: 6.29, 6.30 Mar 9, 2026
@Crabcyborg Crabcyborg modified the milestones: 6.32, 6.33 Jun 16, 2026
@Crabcyborg Crabcyborg modified the milestones: 6.33, 6.34 Jul 14, 2026
@Crabcyborg Crabcyborg modified the milestones: 6.34, 6.35 Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

* @param array $parts Part labels keyed by the show= option value.
* @param stdClass $field The field the parts belong to.
*/
return apply_filters( 'frm_field_parts_for_shortcodes', $parts, $field );

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function apply_filters invoked with 3 parameters, 2 required


The function call is not valid, which will result in a fatal runtime error.

require FrmAppHelper::plugin_path() . '/classes/views/shared/errors.php';

FrmTransLiteListHelper::render_tabs();
$wp_list_table->views();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable $wp_list_table might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants